-
Notifications
You must be signed in to change notification settings - Fork 538
Kittest utility toolbelt #11435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kittest utility toolbelt #11435
Conversation
Web viewer built successfully.
Note: This comment is updated whenever you push a commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really awesome ⭐
crates/viewer/re_viewer/src/viewer_test_utils/app_testing_ext.rs
Outdated
Show resolved
Hide resolved
// Prints the current viewer state. Don't merge code that calls this. | ||
#[allow(unused)] | ||
fn debug_viewer_state(&mut self); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not merge code that calls it? Is it very slow?
If it's called from a test, stdout/stderr only reaches the terminal output if the tests fails.
fn toggle_blueprint_panel(&mut self) { | ||
self.click_label("Blueprint panel toggle"); | ||
} | ||
|
||
fn toggle_time_panel(&mut self) { | ||
self.click_label("Time panel toggle"); | ||
} | ||
|
||
fn toggle_selection_panel(&mut self) { | ||
self.click_label("Selection panel toggle"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a lot nicer to have idempotent commands here, i.e. fn show_blueprint_panel(&mut self, open: bool)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After investigation, it's a bit more complex than I hoped for.
Added tracking issue, will implement as a standalone PR: #11451
fn click_label(&mut self, label: &str) { | ||
self.get_by_label(label).click(); | ||
self.run_ok(); | ||
} | ||
|
||
fn right_click_label(&mut self, label: &str) { | ||
self.get_by_label(label).click_secondary(); | ||
self.run_ok(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate having these helpers to not having to call .run_ok()
after each action… but I wish we had this in kittest instrad. kittest
allows user to buffer up many inputs (e.g. push down some modifiers before clicking), which is powerful, but the price is all these manual calls to .run()
. And realistically, you almost shouldn't test clicking two things in the same frame, since realistically that shouldn't happen in real usage.
In other words: I believe kittest's API is a bit too low-level for most tests.
I don't think there is anything actionable here, except food for thought, and ping @lucasmerlin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually kittest runs all queued up events in seperate steps (so you could click a text edit, type some text and then click a button, and all the events would happen on seperate frames, even if you only call run() once).
But this is really not obvious, so theres definitely room for improvements in that.
And to make it run automatically on clicks, kittest would require a lot of interior mutability, which might make some other apis kind of awkward. But maybe it'd be nice, then we could also hold Nodes across run() calls (so you don't have to re-query them all the time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually kittest runs all queued up events in seperate steps
So why are the .run_ok()
calls even needed here then?
pub async fn test_single_text_document() { | ||
let mut harness = viewer_test_utils::viewer_harness(); | ||
harness.init_recording(); | ||
harness.toggle_selection_panel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this is the problem with non-idempotency. Is this opening or closing the selection panel? 🤷
Related
What